-
Notifications
You must be signed in to change notification settings - Fork 13.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use filtered throttle for battery voltage compensation #9055
Conversation
CI failure is a minor code style issue. If you install astyle locally you can run |
d83211b
to
b1d2097
Compare
Should be fixed now. |
@dagar I'm not quite sure what this next CI error is. |
@MaEtUgR I know you've worked a bit on battery stuff. Can you take a look at this? |
CI error (intermittent land detector issue) can be disregarded as it is not related. |
Note for later, but I think we should have Battery actually subscribe and gather the data it needs. The usage across different parts of the system has become a little inconsistent. |
@MaEtUgR could you take a quick look? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The idea makes sense and the implementation looks good to me.
I would also make the sketchy guess that the estimation improves but more detailed tests would be nice.
I haven't been able to do a proper flight test but I have these two propless bench tests to compare the current implementation to the one in this PR. The battery voltage drop parameter isn't set quite right so you can see that the battery estimate changes depending on the throttle setting. However, in the second log you can see that a sudden change in throttle is no longer able drop the battery estimate before the battery voltage has had time to stabilize. Current implementation: https://review.px4.io/plot_app?log=05a9f3b6-190f-482b-aaa6-9a21677ab2bb Once I am able to do a proper flight test the benefits of the this pull request should be easier to see. |
@dagar @MaEtUgR Here is a log from a flight today using this PR. As you can see the remaining battery estimate is much more stable than in the log from my original post. https://review.px4.io/plot_app?log=20ea78d4-aa8c-4fe9-bceb-daa0f286683c |
@nanthony21 Thanks for the logs, it looks good. I can see in the log that you have current sensing in your setup and I would suggest to you to read https://docs.px4.io/en/config/battery.html and use the current based compensation and fusion with current integration (if you have a fixed size battery) for best results. |
When an internal resistance for the battery has not been selected the firmware uses throttle and the
BAT_V_LOAD_DROP
parameter to compensate for the voltage drop due to load on the battery. The battery voltage has a delayed response to a change in load and it is further filtered to get rid of high-frequency noise. However the throttle value that is used in the calculation can change immediately, this means that when the throttle changes suddenly the compensation will be way off until the filtered voltage has had time to stabilize.This issue can be seen at 18:24.75 in this log:
https://review.px4.io/plot_app?log=53b9938b-ad16-453a-b6f5-e0d5637eba66
When the quadcopter is switched from manual mode to position mode the throttle jumps from 100% to 6%. This causes the battery estimation to drop from ~40% down to 0%. It then slowly recovers back to 40% as the voltage rises.
This pull request replaces the battery voltage drop compensation's raw throttle value with a value that has been filtered at the same time constant as the battery voltage. This prevents the compensation from overshooting during sudden changes of throttle.